-
-
Notifications
You must be signed in to change notification settings - Fork 498
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(project): correctly ignore folders #2147
Conversation
✅ Deploy Preview for biomejs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
.with_formatter() | ||
.with_linter() | ||
.build(), | ||
TraversalMode::Migrate { .. } => vec![], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Migrate should never reach this code path.
let mut ignored = false; | ||
for feature in features { | ||
// a path is ignored if it's ignored by all features | ||
ignored &= self.is_ignored_by_feature_config(path, feature) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use a bitwise operation (bit AND) because a path is ignored only if ALL features ignore that path
@@ -2797,3 +2797,52 @@ fn use_literal_keys_should_emit_correct_ast_issue_266() { | |||
result, | |||
)); | |||
} | |||
|
|||
#[test] | |||
fn should_show_diagnostics_for_linter_ignored_file_when_folder_is_ignored() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a user, I would expect the files inside any build
dir to be ignored if I configured "ignore": ["**/build"]
in the linter
section. So it shouldn't show any diagnostics for the files inside any build
dir.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah my bad, I poorly named the test file. Good catch. It should be formatter diagnostics, NOT linter diagnostics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the snapshot shows the linter diagnostics are printed. Yeah you found it :)
Just noticed now that the diagnostics is showing linting diagnostics. Damn, the fix isn't enough lol |
0561c94
to
21f01af
Compare
4cb639c
to
491c920
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Just left a few comments on naming things.
Summary
Fixes #2131
Maybe #2080 but I'll have to verify it.
Folders were ignored when one single feature was checked, which needed to be updated. Now folders are considered ignored only if ALL FEATURES are ignored for that folder/symbolic link
Test Plan
Added two test cases: